Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add workflow to run conda-store user journey tests #2895

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Jan 6, 2025

Reference Issues or PRs

fixes #2760

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe): CI change

@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 11 times, most recently from 8bb4c2a to 814d7e8 Compare January 7, 2025 22:09
@soapy1
Copy link
Contributor Author

soapy1 commented Jan 8, 2025

depends on conda-incubator/conda-store#1040

@viniciusdc
Copy link
Contributor

Since these tests depend on a live cluster, I suggest extending the already existing local integration tests with this extra check https://github.com/nebari-dev/nebari/blob/main/.github/workflows/test_local_integration.yaml -- since it uses CiRun under the hood you should not be affected by any flakiness of the GH runner in case of constrained resources.

@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 15 times, most recently from 8d941b5 to c108c2f Compare January 16, 2025 21:52
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from e18ebef to 65896b1 Compare January 27, 2025 20:38
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 2 times, most recently from d5f3827 to 8f11a28 Compare January 28, 2025 04:18
@soapy1 soapy1 marked this pull request as draft January 28, 2025 15:19
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 5 times, most recently from 3a6689a to 40df38c Compare February 3, 2025 18:43
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from 40df38c to 42f24f1 Compare February 3, 2025 19:18
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from 903cf0e to bf97857 Compare February 4, 2025 18:23
@viniciusdc
Copy link
Contributor

Hey @soapy1 no rush on this, but could you update us on this? Just considering adding to our next Feb release

@soapy1
Copy link
Contributor Author

soapy1 commented Feb 5, 2025

Heya @viniciusdc I've gotten a bit stuck on this one. I'm not able to find a way to pull an admin token for conda-store from nebari in order to run the tests (full notes in this comment - #2895 (comment)).

Things that I've tried:

  • pulling a service token from the terraform state - this works but is a little janky
  • logging in with the test user and generating a token - this user does not have enough permissions to create an admin token
  • logging in with the root keycloak user to generate a token - this user also doesn't have enough permissions to create an admin token

Other options I've considered (but haven't tried yet):

  • is there a way to seed a new service token for conda store (like a config option to add some service tokens)
  • is there a way to use the keycloak api to add a user with admin permissions, and then generate a token from there

Definitely open to other ideas if you have a suggestion!

@viniciusdc
Copy link
Contributor

viniciusdc commented Feb 5, 2025

I left a comment to your thread there, but basically I think your best options are those two:

is there a way to seed a new service token for conda store (like a config option to add some service tokens)
is there a way to use the keycloak api to add a user with admin permissions, and then generate a token from there

  1. For the first one, conda-store has a service token attribute that could be used, though this would require some extension to allow this to be customizable by the user to include new services:

conda_store_service_token_scopes: Dict[str, Dict[str, Any]] = Field(
alias="conda-store-service-token-scopes"
)

"conda-store-service-token-scopes": {
"cdsdashboards": {
"primary_namespace": "cdsdashboards",
"role_bindings": {
"*/*": ["viewer"],
},
}
},

service-tokens = {
for service, value in var.services : base64encode(random_password.conda_store_service_token[service].result) => value
}

Used for example here:

conda-store-jhub-apps-token = module.kubernetes-conda-store-server.service-tokens.jhub-apps

  1. I think this one might be easier to get going, nebari already has a command to create users from CLI -- nebari keycloak --adduser, which could be used for creating the new user. Right now, there is no way of programmatically changing the group, we can have a script under tests that do this:
try:
    # Get user ID
    users = keycloak_admin.get_users({"username": USER_TO_ADD})
    if not users:
        print(f"User '{USER_TO_ADD}' not found.")
        exit(1)
    
    user_id = users[0]["id"]
    
    # Get group ID
    groups = keycloak_admin.get_groups()
    group_id = next((g["id"] for g in groups if g["name"] == "superadmin"), None)
    
    if not group_id:
        print(f"Group 'superadmin' not found.")
        exit(1)
    
    # Add user to group
    keycloak_admin.group_user_add(user_id, group_id)
    print(f"User '{USER_TO_ADD}' successfully added to 'superadmin' group.")

except KeycloakError as e:
    print(f"Error: {e}")

for the main API KeycloakAdmin class, you might be able to use this function

def get_keycloak_admin_from_config(config: schema.Main):

I wonder if this should be configured directly trough nebari's CLI anyway, we already have root access level credentials in the yaml, so it would not be a security overlook to allow the group in which the user will be created to appear as a custom flag as well....

@soapy1
Copy link
Contributor Author

soapy1 commented Feb 5, 2025

I wonder if this should be configured directly trough nebari's CLI anyway

ya, I was thinking about adding this. Something like nebari keycloak adduser -c nebari-config.yaml --user test password --role superadmin. I think it would be good to have some mechanism so that users can't create a user with more permissions than they ought to be able to.

so it would not be a security overlook to allow the group in which the user will be created to appear as a custom flag as well

could make a v1 of this be something like only root users are allowed to set a role?

@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 5 times, most recently from d9b751c to 66f1ffd Compare February 6, 2025 17:24
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from 550fe91 to 88d000f Compare February 6, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New 🚦
Development

Successfully merging this pull request may close these issues.

[ENH] - add conda-store black-box testing to ci
3 participants